Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] Add ci workflows #6

Merged
merged 4 commits into from
Jun 17, 2024
Merged

[CI] Add ci workflows #6

merged 4 commits into from
Jun 17, 2024

Conversation

red-0ne
Copy link
Collaborator

@red-0ne red-0ne commented Jun 10, 2024

CI Update

This pull request includes the following updates to the CI workflow:

Issue and PR templates

Added the same issue and PR templates as in poktroll.

Workflows

Added reviewdog CI workflow including

  • TODO_IN_THIS_ checks.
  • Standard interface implementation checks.
  • Spell checking
  • Test runner

Makefile targets

Added basic Makefile targets:

  • make test_all
  • make go_lint
  • TODO related targets

@red-0ne red-0ne added the enhancement New feature or request label Jun 10, 2024
@red-0ne red-0ne requested review from Olshansk and okdas June 10, 2024 23:18
@red-0ne red-0ne self-assigned this Jun 10, 2024
# TODO_NB - An important note to reference later
# TODO_DISCUSS_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way for the reviewer of a PR to start / reply to a discussion.
# TODO_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way to start the review process while non-critical changes are still in progress

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[linter-name (fail-on-found)] reported by reviewdog 🐶
# TODO_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way to start the review process while non-critical changes are still in progress

.PHONY: todo_this_commit
todo_this_commit: ## List all the TODOs needed to be done in this commit
grep -n --exclude-dir={.git,vendor,.vscode,.idea} --exclude={Makefile,reviewdog.yml} -r -e "TODO_IN_THIS_"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[linter-name (fail-on-found)] reported by reviewdog 🐶
grep -n --exclude-dir={.git,vendor,.vscode,.idea} --exclude={Makefile,reviewdog.yml} -r -e "TODO_IN_THIS_"�

Copy link
Member

@okdas okdas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @red-0ne!

with:
go-version: "1.22.2"

- name: Run golangci-lint
Copy link
Member

@okdas okdas Jun 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to install and configure it first. I don't remember exactly why, but we don't use the official github action on our main repo. We can try it in this repo! :)

If you don't want to look into this now — I'm happy to pick it up! In that case please remove make go_lint step.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okdas , I'd love to have a linter in this repo. Not sure how to proceed though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@red-0ne you can comment out this step, and I'll set up the golangci-lint next week then, would that work?

@red-0ne red-0ne requested a review from okdas June 14, 2024 04:08
Copy link
Member

@okdas okdas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@red-0ne I've added a commit to remove a dependency on check_go_version make target. If this looks good to you, please go ahead and merge at your convenience.

@red-0ne red-0ne merged commit 7a8056b into main Jun 17, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants